Skip to content

[22.x] win,build: Fix MSVS v17.14 compilation issue on v22#58902

Closed
StefanStojanovic wants to merge 1 commit intonodejs:v22.x-stagingfrom
JaneaSystems:mefi-v22-msvs-fix
Closed

[22.x] win,build: Fix MSVS v17.14 compilation issue on v22#58902
StefanStojanovic wants to merge 1 commit intonodejs:v22.x-stagingfrom
JaneaSystems:mefi-v22-msvs-fix

Conversation

@StefanStojanovic
Copy link
Contributor

@StefanStojanovic StefanStojanovic commented Jun 30, 2025

This prevents updating VS 2022 to the latest version (v17.14), in the release CI. We had similar issues with few VS updates in the past too (17.10 and 17.12 IIRC.

Fixes: #58801

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 30, 2025

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Jun 30, 2025
targos
targos approved these changes Jun 30, 2025
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the embedder string in common.gypi?
I assume this also works with previous versions of MSVC.

@StefanStojanovic
Copy link
Contributor Author

StefanStojanovic commented Jul 2, 2025

Can you update the embedder string in common.gypi?
I assume this also works with previous versions of MSVC.

I've updated the embedder string. Sorry for the delay with the response, but I wanted to test compiling with both 17.13 and 17.12 (on my other machine), and it works on both.

@marco-ippolito marco-ippolito changed the title win,build: Fix MSVS v17.14 compilation issue on v22 [22.x] win,build: Fix MSVS v17.14 compilation issue on v22 Jul 2, 2025
@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 2, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 2, 2025
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 2, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/67784/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 2, 2025

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6640/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 2, 2025

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6640/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 2, 2025

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6640/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 3, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/67793/

@marco-ippolito
Copy link
Member

marco-ippolito commented Jul 3, 2025 •
edited by aduh95
Loading

Can you rebase please? EDIT: nevermind, the CI can rebase on its own. We included #58799 on v22.x-staging so CI should be green

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 3, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/67798/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 3, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/67808/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 3, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/67811/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 3, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/67818/

marco-ippolito pushed a commit that referenced this pull request Jul 3, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
d649e9a 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
@marco-ippolito
Copy link
Member

marco-ippolito commented Jul 3, 2025

Landed in d649e9a

RafaelGSS pushed a commit that referenced this pull request Jul 15, 2025
@StefanStojanovic @RafaelGSS
win,build: fix MSVS v17.14 compilation issue
9c0cb48 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 18, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
b41a339 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 20, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
e30011c 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
@marco-ippolito marco-ippolito mentioned this pull request Aug 20, 2025
Merged
marco-ippolito pushed a commit that referenced this pull request Aug 20, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
03235b9 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 23, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
ff322d1 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 25, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
c46aeef 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 25, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
6e6d99d 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 25, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
5a8e436 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 25, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
1aa21ee 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 26, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
01da496 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
marco-ippolito pushed a commit that referenced this pull request Aug 27, 2025
@StefanStojanovic @marco-ippolito
win,build: fix MSVS v17.14 compilation issue
0dd3a8d 
Fixes: #58801
PR-URL: #58902
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes

@targos targos targos approved these changes

@richardlau richardlau richardlau approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@StefanStojanovic @nodejs-github-bot @marco-ippolito @lpinca @targos @richardlau